Try using MVT for encoding #25
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements MVT (Mapbox Vector Tiles) encoding for timezone data, transitioning from GeoJSON to a more efficient binary format. This change affects both data generation and consumption within the timezone library.
- Replaces GeoJSON marshaling/unmarshaling with MVT encoding/decoding
- Removes mock timezone functionality and associated test cases
- Updates dependencies to include MVT-related packages
Reviewed Changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tzshapefilegen/main.go | Updates data generation to use MVT encoding instead of GeoJSON |
| localtimezone.go | Modifies data loading to decode MVT format instead of GeoJSON |
| localtimezone_test.go | Removes mock timezone tests that are no longer compatible |
| go.mod | Adds required dependencies for MVT support (protobuf and protoscan) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| z.tzData = make(map[string]tzData) | ||
| z.tzids = []string{} | ||
| z.mu.Unlock() | ||
| return errors.New("asdf") |
There was a problem hiding this comment.
The error message 'asdf' is not descriptive. It should clearly indicate what went wrong, such as 'data layer not found in MVT' or 'missing required data layer'.
| return errors.New("asdf") | |
| return errors.New("data layer not found in MVT") |
Copilot uses AI. Check for mistakes.
| "data": fc, | ||
| } | ||
| layers := mvt.NewLayers(data) | ||
| // layers.Simplify(simplify.VisvalingamThreshold(0.0001)) |
There was a problem hiding this comment.
This commented-out simplification code should either be removed if not needed or implemented if required for the MVT encoding functionality.
| // layers.Simplify(simplify.VisvalingamThreshold(0.0001)) | |
| layers.Simplify(simplify.VisvalingamThreshold(0.0001)) |
Copilot uses AI. Check for mistakes.
1 participant
For #1